PR-04: parity DSK + file-io DSK (#475)

[charliecreates]

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

• Issue: Parity denylist group 04: DSK + file-io DSK writers/readback (8 methods) #475
• Head: charlie/issue-454-v2-pr05-dsk-file-io-group04
• Base: charlie/issue-454-v2-pr04-file-io-core-group03

[github-actions]

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

[charliecreates]

This PR expands parity coverage and updates the runner to support DSK/file-io scenarios, but it introduces a potentially breaking change in temp file suffix defaults (.dla → .tmp) and adds large fixed DSK workspace sizes that may be unnecessarily heavy. The new specs currently only assert ok: true, which limits regression detection for JSON shape/value changes. Tightening defaults/sizing and adding minimal structural assertions would make the added coverage substantially more valuable.

Additional notes (2)

• Compatibility | packages/parity-checking/native/src/cspice_runner.c:1310-1310
build_file_io_temp_path() now takes an extension parameter, but the construction combines both extLead and extension. When extension already begins with ., extLead correctly becomes "", so this is fine; however, if callers pass something like ".dsk" vs "dsk", you’ll get two different strings (.dsk vs .dsk) which is okay, but the current API encourages passing either form. More importantly, this function previously always emitted a .dla suffix; changing the default to .tmp can silently break any downstream logic that expects .dla specifically (e.g., tooling, cleanup, file-type inference), especially for DSK writer scenarios.

• Maintainability | packages/parity-checking/native/src/cspice_runner.c:1412-1521
  The new JSON writers (write_dsk_descriptor_json, write_dskb02_json) directly print floating-point values with %.17g. This is generally fine for round-tripping, but it can still introduce cross-platform variance (libc formatting, -0, NaNs/Inf) and can destabilize parity snapshots if you ever start asserting actual numeric values.

Given these are parity scenarios, you’ll likely want stable serialization rules or explicit canonicalization for edge cases before you start expanding expectations beyond ok: true.

Summary of changes

What changed

✅ Parity coverage expanded for DSK + file-io DSK

• Removed dsk.* and file-io.* DSK-related methods from the parity denylist:
  • dsk.dskb02, dsk.dskgd, dsk.dskobj, dsk.dsksrf
  • file-io.dskmi2, file-io.dskopn, file-io.dskw02, file-io.readVirtualOutput
• Added new method specs (schema v2) under:
  • packages/parity-checking/specs/methods/dsk/*@v2.yml
  • packages/parity-checking/specs/methods/file-io/*@v2.yml

🧪 Updated completeness + e2e expectations

• Bumped baseline coverage constants and updated test assertions:
  • covered methods: 106 → 114
  • denylist size: 67 → 59

🧰 CSPICE runner enhancements for new scenarios

• Added minimal DSK geometry constants and helper JSON writers in native/src/cspice_runner.c.
• Enhanced temp file path generation to support arbitrary extensions via build_file_io_temp_path(tag, extension, ...) instead of hard-coding .dla.

🗂️ Added legacy workflows metadata

• Introduced:
  • workflows/legacy/dsk.basic@v1.yml
  • workflows/legacy/file-io.dsk-writers@v1.yml

[charliecreates]

The new DSK constants (DSK_MINIMAL_*) use very large fixed workspace sizes (e.g., 100000, 150000). If these are only used for a minimal single-plate mesh, this is likely far larger than needed and risks wasting memory or masking sizing bugs. It also makes the runner less deterministic across constrained environments.

Since this code lives in a parity runner, it’s worth keeping allocations/data sizes tight and derived from the actual minimal mesh where possible (or at least bounded by a clearly justified constant with rationale).

Suggestion

Reduce the fixed work/voxel sizes to the smallest values that still satisfy CSPICE requirements for your minimal mesh, or compute them based on nv/np with a hard cap.

For example:

#define DSK_MINIMAL_WORKSZ  2000
#define DSK_MINIMAL_VOXPSZ   100
#define DSK_MINIMAL_VOXLSZ   100
#define DSK_MINIMAL_SPXISZ  5000

(or compute worksz from np and nv and clamp).

Reply with "@CharlieHelps yes please" if you want me to add a commit that tightens these constants and adds a short comment explaining the sizing.

[charliecreates]

All new method specs declare result.properties: {} and only assert ok: true. That means these specs won’t catch regressions in the new runner outputs (they’ll only catch hard failures). For parity coverage, that’s a start, but it significantly limits the value of adding these scenarios—especially for DSK bookkeeping/descriptor calls where the output structure is meaningful.

At minimum, it would be good to assert the presence of key fields (or specific known values for the minimal DSK mesh) so that a change in JSON shape or field naming fails fast.

Suggestion

Strengthen expectations to validate shape/keys for the result object. Even without pinning every numeric value, assert required keys exist.

Example (conceptual):

expect:
  ok: true
  result:
    surfce: 0
    center: 399

(or use whatever matcher facilities your harness supports for “has key”).

Reply with "@CharlieHelps yes please" if you want me to add a commit that upgrades these new specs to assert minimal, stable fields for each method.

[charliecreates]

readVirtualOutput@v2.yml case is named read-spk-virtual-output-bytes, but the args use path: parity-read-virtual-output without any extension/namespace, and the new C runner introduces READ_VIRTUAL_OUTPUT_STATES constants. It’s not clear from the spec how the underlying virtual output is produced, whether it’s deterministic across platforms, or whether the path could collide with other tests.

If this relies on temp files, it should ideally go through the same temp-path mechanism (tag + extension) to avoid collisions and to keep cleanup consistent.

Suggestion

Make the virtual-output path unique per run by threading a tag through the runner’s temp-path builder, or by including the test case id in the path. If the DSL supports it, prefer a generated/fixture path token instead of a hard-coded string.

Example direction:

• Change args to pass tag and extension
• Have the legacy call request a temp path via build_file_io_temp_path(tag, ".bin"|".bsp"|...)

Reply with "@CharlieHelps yes please" if you'd like me to add a commit aligning readVirtualOutput with the temp-path builder to avoid collisions.