Skip to content

feat(batch): upfront target validation (v0.2.2)#2

Merged
aborruso merged 2 commits into
mainfrom
feat/batch-upfront-target-check
Jun 9, 2026
Merged

feat(batch): upfront target validation (v0.2.2)#2
aborruso merged 2 commits into
mainfrom
feat/batch-upfront-target-check

Conversation

@aborruso

@aborruso aborruso commented Jun 9, 2026

Copy link
Copy Markdown
Member

Summary

  • batch ora valida subito che --to sia un target supportato per --from, prima ancora di leggere il file CSV
  • Errore chiaro con hint: Run 'openverto targets <from>' to see valid options
  • Bump versione 0.2.1 → 0.2.2

Test plan

  • test_batch_rejects_invalid_target: EPSG 32633 rifiutato come target di 23033, exit code ≠ 0, messaggio corretto
  • test_batch_accepts_valid_target_combination: percorso valido 23033→6706 non bloccato dalla validazione
  • pytest tests/test_offline.py → 19 passed

🤖 Generated with Claude Code

Fail early when --to is not a valid target for --from, before reading the CSV.
Error message includes the `openverto targets <from>` hint.
Adds 2 offline tests covering rejection and valid path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 9, 2026 09:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces an upfront validation step in the batch CLI command to ensure --to is a supported target for the given --from before processing the input CSV, adds offline tests to cover the new behavior, and bumps the project version to v0.2.2.

Changes:

  • Add upfront --from/--to compatibility validation in openverto batch, with a clearer error message and hint.
  • Add two offline CLI tests covering rejection/acceptance of target combinations.
  • Bump version to 0.2.2 and document the release in LOG.md.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
src/openverto/cli.py Adds upfront target validation to batch before reading the CSV.
tests/test_offline.py Adds offline CLI tests for invalid/valid --from/--to pairs.
pyproject.toml Version bump 0.2.10.2.2.
LOG.md Changelog entry for v0.2.2.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/openverto/cli.py Outdated
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@aborruso
aborruso merged commit a91cf7f into main Jun 9, 2026
1 check passed
@aborruso
aborruso deleted the feat/batch-upfront-target-check branch June 9, 2026 09:53
@greptile-apps

greptile-apps Bot commented Jun 9, 2026

Copy link
Copy Markdown

Greptile Summary

This PR adds an upfront validation step to the batch command that checks whether --to is a valid conversion target for --from before reading the CSV file, giving users a clear error message with a hint to run openverto targets <from>. The version is bumped from 0.2.1 to 0.2.2.

  • batch now calls lib_targets(in_e, with_live=False) immediately after parsing EPSG codes and exits with a descriptive error if the target is not in the valid set, avoiding a wasted CSV read.
  • Two new offline tests verify the rejection path (EPSG 32633 refused for source 23033) and the acceptance path (23033→6706 passes validation and proceeds to a faked network call).

Confidence Score: 4/5

Safe to merge; the change is small, well-tested, and does not affect the conversion path.

The validation logic is correct — _die always raises typer.Exit, so valid_targets is never actually unbound at runtime. The only blemish is that _die is typed -> None rather than -> NoReturn, which leaves the variable looking possibly-unbound to static checkers and creates a latent fragility if _die is ever changed.

src/openverto/cli.py — the _die return annotation.

Important Files Changed

Filename Overview
src/openverto/cli.py Adds upfront target validation in batch using lib_targets(in_e, with_live=False); logic is correct but valid_targets appears possibly-unbound to static checkers because _die lacks a NoReturn annotation.
tests/test_offline.py Two new offline tests cover the rejection (invalid target) and acceptance (valid target, faked network) paths; both are well-scoped and use monkeypatching correctly.
pyproject.toml Version bumped from 0.2.1 to 0.2.2; no other changes.
LOG.md Changelog entry added for v0.2.2 describing the new upfront validation and tests.

Fix All in Claude Code

Reviews (1): Last reviewed commit: "fix(batch): use with_live=False for upfr..." | Re-trigger Greptile

Comment thread src/openverto/cli.py
Comment on lines +399 to +403
try:
valid_targets = lib_targets(in_e, with_live=False)
except KeyError:
_die(f"EPSG {in_e} is not IGM-supported; run 'openverto systems'", EXIT_NOT_FOUND)
if out_e not in {r["epsg"] for r in valid_targets}:

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Potentially-unbound variable visible to static analysis

valid_targets is only assigned inside the try block. If KeyError is raised and _die is called, execution never reaches the if statement below — but _die is typed -> None, not -> NoReturn. Static type checkers (mypy/pyright) will therefore report valid_targets as possibly unbound on the next line, and any future change to _die that makes it return normally (e.g. for testing) would produce a NameError. Annotating _die as -> NoReturn (or restructuring with an else block) is the minimal fix.

Fix in Claude Code

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.

2 participants