Honor annotation_overwrite in per-image upload_dataset (REQ-182)

[tonylampada]

Description

Customer problem: re-running roboflow upload ./dir with a COCO file that has no annotations looked like it wiped the boxes on images already in the project, and the CLI gave no way to choose (O8).

What this PR solves:

• CLI: roboflow upload ./dir --annotation-overwrite exited with No such option, and roboflow image upload ./dir --annotation-overwrite (and upload_dataset(..., annotation_overwrite=True)) ignored the flag outside the zip flow. Both now replace annotations on duplicates.
• Log: a duplicate whose annotations the API kept used to print annotations = WARN: already annotated. It now prints annotations = kept existing (pass --annotation-overwrite / annotation_overwrite=True to replace).

Already fixed server-side: the wipe itself. Since roboflow#12836 (internal link removed) the API returns 409 on a duplicate that is already annotated unless overwrite is set, so the boxes survive (confirmed on staging below).

Type of change

• Bug fix (non-breaking)

How has this change been tested?

• CI green.
• New automated tests added: pytest tests/test_project.py -k PerImage 4/4, tests/cli/test_image_handler.py -k three_states (now covers the upload alias). Full suite 1234 passed.
• Staging (throwaway project, 2 images, per-image flow): fresh upload saves the boxes; re-upload with an empty COCO keeps them (kept existing …); re-upload with --annotation-overwrite replaces them. On main: the log says WARN: already annotated, image upload --annotation-overwrite leaves the boxes untouched, and upload --annotation-overwrite exits 2.
• Review: one jarbas round. It found that the log hint named only the SDK kwarg, that the log line had lost its timing, that the alias help didn't match image upload, and that the classification default had no test. All four are fixed and staging re-ran green after the fix.
• Evidence (terminal sessions + API reads before/after each step): https://gist.github.com/tonylampada/19ad33ebaadf2923666fb7fa143b95f4

Any specific deployment considerations

None. SDK release only.

🤖 Generated with Claude Code

[tonylampada]

/jarbas review

[jarbas-roboflow]

PR Review: Honor annotation_overwrite in per-image upload_dataset (REQ-182)

• Repository: roboflow/roboflow-python
• PR: #552 (OPEN)
• Branch: bc/LEN-122 → main
• Author: Tony Lâmpada (tonylampada)
• Head SHA: dfde5263ee7428924abadbee234e301cd7efb5c7
• Base SHA: d5338aac41ed8781102924de97f9a252adbbd8c7
• Change size: 5 files, +101 / −18
• Review date: 2026-10-14

📋 Summary

This PR correctly extends annotation_overwrite from the async zip-upload path to the legacy per-image dataset-upload path. Both supported CLI forms now retain the option's three states, Workspace.upload_dataset resolves the default based on project type, and the resolved value reaches the existing annotation API's overwrite query parameter.

The duplicate-image message is also more actionable and accurately describes a 409 “already annotated” response as preserving the existing annotation. I found no merge-blocking issue in the current head.

🏗️ Architectural Review

The change follows the repository's existing layers and keeps responsibility in the right places:

flowchart LR
    CLI[image upload] --> Args[CLI args]
    Alias[top-level upload] --> Args
    Args --> Workspace[Workspace upload_dataset]
    Workspace --> Default[Resolve project default]
    Default --> Project[Project save_annotation]
    Project --> API[Annotation API overwrite]

Loading

• The primary and compatibility CLI commands expose the same tri-state option and converge on the existing handler (primary option, alias option and forwarding). This preserves the documented backwards-compatibility alias pattern.
• Default resolution remains in Workspace, where project type is available, and now applies uniformly before either upload path branches (default resolution).
• The per-image implementation reuses the established Project.save_annotation contract rather than adding a second API mechanism (forwarding call). Dependency direction remains CLI → workspace → project → adapter.
• The existing API adapter continues to own 409 normalization, so the workspace only chooses the user-facing message and does not duplicate transport-level parsing (duplicate-result logging).

🐛 Code Quality Issues

No critical, warning, or minor correctness findings.

The public default remains backward compatible for non-classification projects (None resolves to False), classification projects receive the API-required True, and an explicit False is preserved because only None triggers defaulting.

💡 Recommendations

• Add one focused regression case for a classification project with annotation_overwrite=False. The current tests prove the classification default and explicit True, while this case would pin the subtle but important distinction between “unspecified” and “explicitly disabled” at the workspace boundary. This is test hardening, not a merge blocker; the implementation already handles the case correctly (default branch, current classification test).

🧪 Tests

The added and updated tests meaningfully cover the changed behavior:

• The CLI test proves both roboflow image upload and the hidden top-level roboflow upload alias preserve omitted, enabled, and disabled option states before entering the handler (three-state CLI test).
• The per-image tests prove a fresh image annotates without an overwrite query, a duplicate without overwrite retains existing annotations and emits the new message, explicit overwrite adds overwrite=true, and classification defaults to overwrite (per-image behavior tests).
• These tests exercise the real Project.save_annotation and adapter URL/409 handling while mocking only dataset parsing, image upload, and project lookup, which gives useful integration coverage of the changed call chain.

I could not independently execute the targeted suite in the prepared environment because its system Python does not have pytest installed. The PR description reports the focused tests and full suite passing; that claim was not re-verified locally during this review.

⚖️ Verdict — ✅ Approve

The implementation is small, correctly layered, backward compatible, and directly covered across the CLI and per-image annotation path. The additional explicit-False classification test would strengthen the contract but is not required for merge.

---

Generated by Codex via roboflow-review-pr. Review is advisory; verify before merging.

Review completed: 2026-10-14 16:58:32 UTC