PR-16: migrate ids/names to v2 (#454)

[charliecreates]

Stack step PR-16 in the issue #454 migration chain.

• Issue: Simplify CSPICE parity runner from first principles #454
• Head: charlie/issue-454-v2-pr17-migrate-ids-names-v2
• Base: charlie/issue-454-v2-pr16-migrate-kernels-v2

[charliecreates]

The migration to schemaVersion: 2 is mechanically consistent, but it introduces risk by hardcoding kernels (potentially diverging from the v1 uses workflow) and by keeping weak assertions (expect.ok: true only), which can allow mapping/normalization regressions to slip through. contract.result being an empty object also reduces the usefulness of v2 contracts for validation/tooling. Consider centralizing shared setup/workflow and asserting concrete outputs to make parity checks meaningful.

Summary of changes

Summary

Migrated ids-names parity specs from v1 to v2 YAML schema for several methods:

• Replaced legacy top-level fields (id, kind, contractMethod, canonicalMethod, uses) with v2 structure:
  • schemaVersion: 2
  • manifest: { id, kind }
  • contract: { contractMethod, canonicalMethod, aliases, result }
  • setup.kernels (now explicitly includes $FIXTURES/frames-minimal and $FIXTURES/body399-char-foo.tpc)
  • workflow.steps using op: invokeLegacyCall
• Preserved existing cases (IDs, args, and expect.ok: true) while bumping spec identifiers from @v1 to @v2.

Files migrated:

• bodc2n, bodc2s, boddef, bodfnd, bodn2c, bods2c, bodvar

[charliecreates]

All migrated specs hardcode the same setup.kernels list. That may be correct for most of these methods, but it also subtly changes the runtime environment vs v1, where the kernel set was inherited indirectly via uses: workflows/legacy/ids-names.basic@v1. If any of these methods require additional kernels (or fewer kernels), this can create false parity signals (tests passing due to extra data being available) or flaky behavior if the legacy workflow used a different set.

Suggestion

Factor this duplicated kernel selection into a shared v2 workflow (or a reusable fixture preset) equivalent to workflows/legacy/ids-names.basic@v1, and reference it from each spec so the kernel environment is consistent and centrally maintained. For example, introduce something like setup: { uses: [workflows/legacy/ids-names.basic@v2] } (or whatever v2 supports), or at least a shared setup.kernels include. Reply with "@CharlieHelps yes please" if you'd like me to add a commit implementing a shared v2 workflow/preset and updating these specs to reference it.

[charliecreates]

contract.result is declared as an object with empty properties. If the v2 runner uses contract.result for result-shape validation, documentation, or downstream tooling, this makes the contract non-informative and may hide regressions (e.g., missing fields) because nothing is asserted about output structure.

Suggestion

If these methods have a known output shape, encode it in contract.result (even minimally: required fields and basic types). If they intentionally return a legacy scalar/tuple, consider setting type accordingly (or using additionalProperties: true with a comment-free schema) so the contract reflects reality. Reply with "@CharlieHelps yes please" if you'd like me to add a commit that aligns contract.result schemas with the actual outputs for these ids-names.* methods.

[charliecreates]

All cases only assert expect.ok: true and do not validate returned values (e.g., that bodn2c('EARTH') returns 399, or that bodc2n(399) returns the expected canonical name). For parity checking, this can miss the primary regressions these methods are prone to (wrong mapping, normalization differences, or differing "not found" behavior).

Suggestion

Extend expect to assert the returned value(s), not just success. For example, add an expect.result/expect.value field (whatever the v2 spec supports) for known inputs like EARTH -> 399, 399 -> 'EARTH' (or expected casing), and for unknowns assert the correct sentinel/flag (found=false, empty string, 0, etc.) rather than ok: true. Reply with "@CharlieHelps yes please" if you'd like me to add a commit that updates these cases to assert concrete outputs based on the legacy behavior.

[charliecreates]

contract.result is declared as an empty object schema (properties: {}) for a method that likely returns a scalar/primitive (e.g., integer code). If the v2 contract schema is used for validation or documentation, this placeholder result shape can mask regressions and makes the spec less meaningful.

Suggestion

Update contract.result to match the actual return type for each method if the framework uses it (e.g., type: integer for code-returning methods, type: string for name-returning methods, etc.). If the result is intentionally unspecified, prefer an explicit permissive schema (e.g., omit result if allowed, or use something like {} / additionalProperties: true depending on the schema) rather than implying an object with no properties.

Reply with "@CharlieHelps yes please" if you'd like me to add a commit updating the result schemas for these ids/names methods based on their known return types (or adjusting to the preferred 'unspecified' pattern for v2).

[charliecreates]

The v2 specs always use workflow.steps: - op: invokeLegacyCall. This keeps parity coverage for legacy behavior but also means these tests won't catch issues in a non-legacy execution path (if one exists in v2). If the migration goal is parity plus forward-path validation, these should either parameterize the workflow or add additional cases/specs that invoke the modern path.

Suggestion

If the framework supports it, add a second workflow path (or separate spec) that invokes the v2/native implementation (e.g., invoke vs invokeLegacyCall) and compare outputs, or at least ensure there is a dedicated suite for non-legacy execution. Reply with "@CharlieHelps yes please" if you'd like me to add a commit that introduces a parallel non-legacy workflow/specs where available.

[github-actions]

orrery preview: https://pr-516.orrery-c4f.pages.dev/