PR-05: parity EK open/query/read (#476)

[charliecreates]

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

• Issue: Parity denylist group 05: EK open/query/read surface (11 methods) #476
• Head: charlie/issue-454-v2-pr06-ek-open-query-read-group05
• Base: charlie/issue-454-v2-pr05-dsk-file-io-group04

[github-actions]

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

[charliecreates]

New EK specs add coverage but currently don’t validate returned result shapes (many result schemas are empty objects), which can hide parity regressions. The ekgi error test may be brittle if the query isn’t tightly coupled to a fixture-guaranteed double column. The updated completeness baselines work, but relying on hard-coded counts increases maintenance churn and weakens the guard’s signal over time.

Additional notes (1)

• Maintainability | packages/parity-checking/src/guards/validateCompleteness.ts:7-8
  The completeness guard uses hard-coded baseline counts (BASELINE_CANONICAL_METHOD_COVERAGE, MAX_BASELINE_DENYLIST_SIZE). Bumping them is necessary, but this pattern tends to create churn across PRs and makes the guard less informative (it becomes “update the constant” rather than “prove why coverage changed”). It also risks masking accidental coverage drops if someone updates the baseline incorrectly.

A more resilient approach is to compute and snapshot the baseline (or derive it from the denylist/specs) in a single place, or at least include a comment/link to the PR/issue that justified the bump.

Summary of changes

What changed

• Expanded EK method parity coverage (v2) by adding new method specs under packages/parity-checking/specs/methods/ek/:
  • ekcls@v2.yml, ekfind@v2.yml, ekgc@v2.yml, ekgd@v2.yml, ekgi@v2.yml, eknseg@v2.yml, ekntab@v2.yml, ekopn@v2.yml, ekopr@v2.yml, ekopw@v2.yml, ektnam@v2.yml
• Reduced the parity denylist by removing the above EK methods from:
  • packages/parity-checking/catalogs/parity-denylist.json
  • packages/parity-checking/catalogs/parity-denylist.ts
• Updated completeness baselines and tests to reflect increased canonical coverage and a smaller denylist:
  • BASELINE_CANONICAL_METHOD_COVERAGE: 114 → 125
  • MAX_BASELINE_DENYLIST_SIZE: 59 → 48
  • Corresponding expectation updates in integration/e2e/coverage tests
• Added a legacy workflow definition packages/parity-checking/workflows/legacy/ek.basic@v1.yml to provide EK fixture coverage for query/read methods.

[charliecreates]

The new EK specs all declare result: { type: object, properties: {} }, which effectively stops the parity framework from validating any returned data shape. That makes these tests closer to “did it run” smoke tests than true parity checks, and it can hide real regressions (e.g., wrong handle IDs, row counts, found flags, returned string values). At minimum, a few of these calls should assert stable, meaningful fields (even if partial) to ensure we’re exercising the parity contract.

Suggestion

Consider tightening contract.result for EK methods to assert at least the minimal stable fields you expect back (e.g., row count / found, output value fields, handle IDs). For example, if ekfind returns a found boolean or row count, encode that in the schema and add expect assertions in the case.

If you want, reply with "@CharlieHelps yes please" and I can add a commit that updates a couple of these specs (e.g., ekfind, eknseg, ekntab) to validate minimal result fields and add expectations accordingly.

[charliecreates]

This case is intended to validate error parity (SPICE(INVALIDTYPE)), which is good. However, the args look identical in shape to ekgd@v2 and ekgc@v2 (query string + 0/0/0). If the underlying EK fixture or query changes, this could start failing for reasons unrelated to INVALIDTYPE (e.g., missing column/table). It would be more robust to ensure the query unambiguously targets a known double column and that the call under test is definitively the integer getter, so the only failure mode is type mismatch.

Suggestion

Make the failing query more explicit/robust by referencing a column that is guaranteed (by fixture design) to be double, and consider adding a brief precondition via workflow (or a comment in the spec) that the table/column exists in ek-fixture.bes. If you want, reply with "@CharlieHelps yes please" and I can add a commit that adjusts the query to a more fixture-stable double column name (if available in the fixture) and documents the assumption in the spec.

[charliecreates]

These EK read/query specs define result as an object with empty properties and do not specify any expect payload for successful cases. That risks turning the new coverage into a “smoke test” that only checks non-crash behavior, which can miss parity regressions in actual returned values (especially for ekgc/ekgd where result contents matter). If the framework supports it, assert at least one stable field/value (or a snapshot-like shape) so the tests validate correctness, not just execution.

Suggestion

Add minimal expect assertions for the successful cases—e.g., verify returned value(s) are present and match known fixture data.

For example (illustrative):

cases:
  - id: reads-event-string
    args: ["SELECT EVENT FROM CASSINI_NOISE_EVENTS", 0, 0, 0]
    expect:
      ok: true
      value:
        event: "<known event from fixture>"

If the actual return shape is a list/tuple, assert the first element or a count plus a representative value.

Reply with "@CharlieHelps yes please" if you’d like me to add a commit that strengthens these EK specs with concrete expectations based on the fixture.