PR-15: migrate kernels to v2 (#454)#515
PR-15: migrate kernels to v2 (#454)#515charliecreates[bot] wants to merge 1 commit intocharlie/issue-454-v2-pr15-migrate-kernel-pool-v2from
Conversation
|
orrery preview: https://pr-515.orrery-c4f.pages.dev/ |
There was a problem hiding this comment.
The migration to schemaVersion: 2 is structurally consistent, but it likely reduces coverage by declaring an empty contract.result schema for methods that return data. Also, replacing uses: workflows/legacy/kernels.basic@v1 with a bare invokeLegacyCall step may change test semantics if the legacy workflow performed critical kernel-pool setup/teardown. Addressing these will improve regression detection and avoid subtle behavioral drift.
Summary of changes
Kernel parity specs migrated to schema v2
This PR replaces the legacy @v1 method specs with new schemaVersion: 2 equivalents for the kernels API:
- Deleted
packages/parity-checking/specs/methods/kernels/*@v1.yml - Added
packages/parity-checking/specs/methods/kernels/*@v2.yml
New v2 structure
Each new spec now follows the v2 layout:
schemaVersion: 2manifest(id,kind)contract(contractMethod,canonicalMethod,aliases,result)workflow.stepsusingop: invokeLegacyCall- Existing
casesmigrated over with the sameargs,setup, andexpectcontent
Affected methods: kernels.furnsh, kernels.kclear, kernels.kdata, kernels.kinfo, kernels.kplfrm, kernels.ktotal, kernels.kxtrct, kernels.unload.
| contract: | ||
| contractMethod: kernels.kdata | ||
| canonicalMethod: kernels.kdata | ||
| aliases: [] | ||
| result: | ||
| type: object | ||
| properties: {} | ||
| workflow: |
There was a problem hiding this comment.
All migrated @v2 specs define contract.result as an object with empty properties: {}. For methods like kernels.kdata, kernels.kinfo, kernels.kplfrm, kernels.ktotal, and kernels.kxtrct, the legacy call returns structured data; using an empty object schema removes a lot of validation value and can allow regressions to slip through (tests may only assert ok: true).
Suggestion
Tighten contract.result schemas where the method returns data (at least basic shape/types), and keep {} only for methods that truly return no payload. For example:
contract:
result:
type: object
required: [data]
properties:
data:
type: object(or whatever the real response envelope is for these specs). If you'd like, reply with "@CharlieHelps yes please" and I can add a commit that updates the result schemas for the kernel methods that return data.
| workflow: | ||
| steps: | ||
| - op: invokeLegacyCall | ||
| cases: |
There was a problem hiding this comment.
workflow.steps is now just - op: invokeLegacyCall, but the v1 specs explicitly referenced workflows/legacy/kernels.basic@v1 via uses. If the legacy workflow previously included important setup/teardown (e.g., kernel pool isolation, automatic kclear, fixture loading conventions), relying on a generic invokeLegacyCall could change semantics across these tests.
Suggestion
Confirm invokeLegacyCall composes the same legacy workflow behavior as workflows/legacy/kernels.basic@v1. If it does not, encode the missing pieces explicitly in the v2 workflow (e.g., add steps that mirror the legacy workflow or reference an equivalent v2 workflow abstraction if available).
Reply with "@CharlieHelps yes please" if you'd like me to add a commit that reintroduces the equivalent workflow semantics (either by referencing the correct workflow or by expanding the necessary steps) once you confirm the intended v2 workflow contract.
Stack step PR-15 in the issue #454 migration chain.