fix(tags): report server 4xx rejections instead of masking them

[ryanhayame]

This PR does two independent, forward-compatible things: (1) makes the SDK report server errors honestly instead of masking/retrying them, and (2) sends a sampleIndex so lists of media keep their logged order in the UI.

Both are safe to merge before any server/DB change — see "Merge safety" at the bottom.

---

Part 1 — report server 4xx rejections instead of masking them

Problem. On a non-2xx write, iface._try retried everything (incl. 4xx) with backoff, then either silently no-op'd (/api/runs/tags/update — a failed run.add_tags(...) looked like it worked) or raised a misleading ConnectionError("...Check connection...") (/api/runs/create — blamed the network). The server's real reason lived only in a buried warning log. This first surfaced with the server's new "a run can have at most one group:* tag" 400.

Change. Surface the server's reason; don't rewrite the caller's request.

• iface.py: new PlutoRequestError (+ _server_error_message, parses the JSON error field). _try treats permanent 4xx as terminal (no retry, no wasted backoff) and, when raise_on_error=True, raises PlutoRequestError with the server's message. Transient 4xx — 401/408/429 (RETRYABLE_STATUS_CODES) — and 5xx/network stay retryable (auth races, timeouts, rate limits), mirroring query.py.
• op.py: create/resume pass raise_on_error=True → a rejection raises RuntimeError("Failed to create run: <server reason>"); a genuine unreachable server still raises ConnectionError. add_tags/remove_tags surface a rejection at WARNING instead of DEBUG.
• sync/process.py: the sync-subprocess uploader (_post_with_retry, the primary run.add_tags path) uses raise_for_status() and applies the same terminal-4xx / retryable-401/408/429 policy, so the sync loop logs the real reason instead of a bare 400.

No client-side tag rewriting — if a caller sends two group:* tags, the request fails and the server's reason is reported (we do not silently drop one).

Part 2 — sampleIndex so media lists keep logged order

Problem. pluto.log({"eval/img": [img0, img1, img2]}, step=s) came back out of order in the UI because the server sorts a step's files by fileName and nothing recorded each image's position.

Change (SDK side). Each object in the POST /files files: [...] array now carries a non-negative, 0-based integer sampleIndex = the file's position within a single (logName, step) log() call.

• A list of N media → 0..N-1; a single (non-list) media → 0.
• Transparent — no new public API. The index is derived purely from list position via enumerate(), captured at log() time (op.py, where order is known) and threaded through the sync DB to the payload:
  • op.py: enumerate items in the log loop → _process_log_item_sync → _enqueue_file_sync → SyncProcessManager.enqueue_file
  • sync/store.py: new sample_index column (additive migration, defaults 0), persisted on enqueue and read back into FileRecord
  • sync/process.py: _get_presigned_urls adds sampleIndex to each entry
  • api.py: make_compat_file_v1 (legacy builder) enumerates each logName's list
• Deriving from the stored position (not by numbering the upload batch) is deliberate — sync batches can split or mix steps, so batch-numbering would be wrong.

Tests

• tests/test_iface_errors.py: _try doesn't retry a 400 and raises PlutoRequestError with the server message (not ConnectionError); persistent 5xx/401 retry then raise with the real status_code; transient 401 that clears on retry succeeds; network error returns None; the sync uploader doesn't retry a 400 and surfaces the reason.
• tests/test_sync_process.py: store round-trip (sample_index list → 0..N-1, default 0); presign payload carries sampleIndex; make_compat_file_v1 numbers a list 0..N-1 and a single media 0.

Ruff + mypy clean on changed files.

Merge safety (verified against server-private)

Safe to merge before the backend/DB changes:

• Part 1 is pure client-side behavior — no backend dependency. If the server doesn't yet enforce the group:* rule, there's simply no 400 to surface; behavior is unchanged.
• Part 2 adds an extra JSON field. The ingest upload endpoint (ingest/src/models/files.rs, FileInput) has no #[serde(deny_unknown_fields)], so serde silently ignores the unknown sampleIndex — same pattern as the already-shipped caption field. Until the backend adds sample_index (column + sort), media just keeps its current fileName sort; after, ordering works. No coordinated deploy required.

🤖 Generated with Claude Code

[gemini-code-assist]

Code Review

This pull request introduces PlutoRequestError and a helper function _server_error_message to capture and surface human-readable server validation errors (such as 4xx responses) instead of raising generic connection errors. It updates the ServerInterface and the sync uploader to immediately halt retries on 4xx client errors, while adding comprehensive unit tests to verify this behavior. The reviewer suggests an improvement to extract and propagate the actual HTTP status code from the error message when raising PlutoRequestError on persistent server errors (e.g., 5xx errors) after exhausting retries, along with a corresponding test assertion to verify this behavior.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

[ryanhayame]

@cursor review

[cursor]

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit 63bc5a2. Configure here.}