Skip to content

Reject mismatched batch indices before returning granule groups - #231

Open
sylvesterkaczmarek wants to merge 1 commit into
nasa:developfrom
sylvesterkaczmarek:fix/reject-misaligned-batch-indices
Open

sylvesterkaczmarek wants to merge 1 commit into
nasa:developfrom
sylvesterkaczmarek:fix/reject-misaligned-batch-indices

Conversation

@sylvesterkaczmarek

Copy link
Copy Markdown

GitHub Issue: #230

Description

Fixes #230.

Require matching input lengths in _group_batch_indices by using strict zip. The filename parser may omit unrecognized names while callers retain the full filename/STAC-item list. Non-strict zip then associates indices with the wrong granules and silently drops trailing items.

A mismatch now raises ValueError before any grouped result is returned to the CLI or adapter. The helper's documentation explains this invariant. Valid grouping order and object identity are unchanged, as are the parser's existing invalid-name behavior, date/scan calculation, and empty-input behavior. This does not reintroduce the broader parser/URL changes excluded from #197.

Local test steps

Added 12 cases covering shorter and longer lists, one empty list, invalid filenames at the beginning/middle/end, all-invalid CLI input, valid ordering/object identity, and actual adapter invocation with synthetic STAC catalogs. Eleven regression cases fail on unchanged code; the valid-grouping control passes.

With the fix:

  • python -m pytest --cov=batchee --cov-branch --cov-report=term-missing -q: 20 passed, including the existing paged and unpaged Harmony CLI test paths.
  • ruff check .: passed.
  • Ruff formatting checks on the changed Python files, Python compilation, and git diff --check: passed.
  • mypy batchee: 15 existing errors. The identical diagnostics were reproduced against a separate unchanged-upstream worktree using the same environment. No typing/dependency cleanup is included here.

Overview of integration done

The existing Harmony CLI tests read local paged/unpaged catalogs and verify the generated batch catalogs and items. New failure cases invoke ConcatBatching.invoke() using real in-memory pystac objects and confirm that a bad mapping raises without mutating the input items/assets. No live granules were downloaded and no deployed Harmony service was used.

Tested on macOS with Python 3.12.11 against develop at 86cec9b2637d0d7bd471ab88c7a5496fda4329b0.

PR Acceptance Checklist

  • Unit tests added and passing.
  • Deployed-service integration testing. Local CLI/adapter coverage is described above.
  • CHANGELOG.md updated.
  • Helper documentation updated.

@sylvesterkaczmarek

Copy link
Copy Markdown
Author

Any feedback on the batch-index validation? Let me know what needs changing before review.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mismatched batch indices silently drop or misassign granules

1 participant